Skip to content

Conversation

@brendandahl
Copy link
Collaborator

Return the promise from handleSleep so wasm is suspended until each idb operation is done.

Fixes #19989

@sbc100
Copy link
Collaborator

sbc100 commented Aug 7, 2023

I guess this only really matters with ASYNCIFY=2?

@brendandahl
Copy link
Collaborator Author

I guess this only really matters with ASYNCIFY=2?

Yeah, asyncify=1 has its own mechanism in handleSleep to start things back up again. I've contemplated unifying asyncify 1 and 2, where we wouldn't use handleSleep or handleAsync in js library functions, but we would instead always expect a promise back from an async function and resume on that promise. Currently, asyncify=1 supports doing either sync or async from an import and if we went to the promise approach, a function would no longer be able to do the sync resume. I suppose if the user really wanted sync they could import a sync only version though. Maybe, @kripken has thoughts on this?

@kripken
Copy link
Member

kripken commented Aug 7, 2023

Unifying 1 and 2 sounds good in general. I don't have a feeling for how limiting it would be to not allow sync imports, though. Do we have examples of that in the test suite or elsewhere?

@brendandahl
Copy link
Collaborator Author

I don't think it will really be limiting, more that it could affect performance. Currently with asyncify=1, a JS function that is calling handleSleep can synchronously call the wakeUp callback so that asyncify=1 won't unwind. If we go the promise route, asyncify=1 will always unwind during a js async import.

@brendandahl
Copy link
Collaborator Author

So far I've been unable to reproduce the test failing locally. I'm going to try with a different version of chrome...

@kripken
Copy link
Member

kripken commented Aug 9, 2023

I don't think it will really be limiting, more that it could affect performance. Currently with asyncify=1, a JS function that is calling handleSleep can synchronously call the wakeUp callback so that asyncify=1 won't unwind. If we go the promise route, asyncify=1 will always unwind during a js async import.

Could we not return a resolved promise and not unwind, something like that? (I admit I don't totally follow this, though, so I'm probably missing something... is there an example of a particular import that could get slower due to this?)

Return the promise from handleSleep so wasm is suspended until each idb
operation is done.

Fixes emscripten-core#19989
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Functions with out params don't get them correctly set from JS with ASYNCIFY=2

3 participants